Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor for wgpu v23 compatibility with an example of wgpu device sharing #211

Merged
merged 20 commits into from
Dec 5, 2024
Merged

Refactor for wgpu v23 compatibility with an example of wgpu device sharing #211

merged 20 commits into from
Dec 5, 2024

Conversation

AsherJingkongChen
Copy link
Contributor

@AsherJingkongChen AsherJingkongChen commented Oct 29, 2024 •
edited
Loading

Related Issues/PRs

Checklist

  • Confirmed that cargo xtask test --ci script has been executed.
  • Made sure the book is up to date with changes in this PR.
  • Checked for any updates or changes in dependencies.

Changes

1. Modified entry_point Type

  • Change: Updated the type of entry_point from &str to Option<&str>.
  • Reason: Clippy's suggestion

2. Updated WgpuDevice::Existing Enum Type

  • Change: Altered the type of WgpuDevice::Existing from wgpu::Id<wgpu::Device> to u32.
  • Reason: After the changes introduced in gfx-rs/wgpu#6134, wgpu::Id no longer exists. Using a u32 ensures a unique identifier for WgpuDevice::Existing.

Note: We avoid using pointers as identifiers due to the possibility of reallocation at the same memory address. To maintain uniqueness, a monotonically increasing value is now used internally.

Examples

  1. Added device_sharing Example
    • Description: This example demonstrates the device sharing feature.
    • How to Run: Use the following command to run the example: 
      cargo run --example device_sharing --features wgpu

@AsherJingkongChen AsherJingkongChen mentioned this pull request Oct 29, 2024
Draft
4 tasks
@AsherJingkongChen AsherJingkongChen changed the title Refactor for wgpu v23 compatibility and add an example Refactor for wgpu v23 compatibility and add an example of wgpu device sharing Oct 29, 2024
@AsherJingkongChen AsherJingkongChen changed the title Refactor for wgpu v23 compatibility and add an example of wgpu device sharing Refactor for wgpu v23 compatibility with an example of wgpu device sharing Oct 29, 2024
@AsherJingkongChen
Refactor for wgpu v23 compatibility
a3c4e33 
* Change type of `entry_point` from `&str` to `Option<&str>`
* Change enum type of `WgpuDevice::Existing` from `wgpu::Id<wgpu::Device>` to `usize`
@AsherJingkongChen
Add example of using existing wgpu device
7f689c0
@AsherJingkongChen
Format
13dd59d
@AsherJingkongChen
Patch changes for SPIR-V compiler
bfa5032
@AsherJingkongChen
Make WgpuDevice::Existing unique
dd04e6c 
* Added auto-increment counter in init_existing_device
* Added comments of usage
@ArthurBrussee ArthurBrussee mentioned this pull request Nov 22, 2024
Merged
@ArthurBrussee
Copy link
Contributor

In theory now that the OOB behaviour has been fixed this should work now, if you have time to re test on metal would be amazing :) Thank you!

@AsherJingkongChen

This comment has been minimized.

Sign in to view
@nathanielsimard
Copy link
Member

In linux CI, there is a segfault without message, would it come from CUDA? I can't resolve it now.

I don't think the CI is building CUDA.

@nathanielsimard
Copy link
Member

@AsherJingkongChen If you merge main, does it fix the CI?

@AsherJingkongChen AsherJingkongChen mentioned this pull request Dec 4, 2024
Closed
@AsherJingkongChen
Copy link
Contributor Author

AsherJingkongChen commented Dec 4, 2024 •
edited
Loading

I don't get it why did it pass CI. I just updated feature flags. LOL.

@AsherJingkongChen AsherJingkongChen marked this pull request as ready for review December 5, 2024 05:22
@nathanielsimard nathanielsimard merged commit 13fc62c into tracel-ai:main Dec 5, 2024
5 checks passed
@syl20bnr
Copy link
Member

syl20bnr commented Dec 5, 2024

I don't get it why did it pass CI. I just updated feature flags. LOL.

This is because the CI errors was caused by the spirv feature which is not yet fully functional on CPU.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArthurBrussee ArthurBrussee ArthurBrussee left review comments

@nathanielsimard nathanielsimard nathanielsimard approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AsherJingkongChen @ArthurBrussee @nathanielsimard @syl20bnr